Host GPIO improvements #2737
Host GPIO improvements #2737
Conversation
All the approriate gpio_xxx functions now call check_gpio_param. This provides an easy way for a project to add simple range checking by defining a final version of check_gpio_param, which whatever error mechanism it chooses if an invalid value is passed. Refs raspberrypi#2736
This allows them to be easily overridden by user code for testing purposes. Refs raspberrypi#2736
This prevents lots of compiler warnings if the default warning level is increased. Refs raspberrypi#2736
Some RP2350 GPIO_FUNC_ enums don't match the values defined in the rp2350 hardware header but the actual values shouldn't matter if only the enums are used (i.e. no magic numbers). Refs raspberrypi#2736
| #include "hardware/irq.h" | ||
|
|
||
| enum gpio_function { | ||
| typedef enum gpio_function { |
There was a problem hiding this comment.
Looks like this is a "superset" of RP2040's and RP2350's gpio_function enums? That makes sense, but I'm not sure if there's any point in having GPIO_FUNC_XIP and GPIO_FUNC_XIP_CS1 both set to 0?
There was a problem hiding this comment.
Yes, it's a superset. I set GPIO_FUNC_XIP_CS1 to the same value as GPIO_FUNC_XIP as each enum only exists on one platform (rp2040/rp2350b) so I don't think there should be a problem but it's not something I have strong feelings on either way.
| GPIO_IN = 0u, ///< set GPIO to input | ||
| }; | ||
|
|
||
| enum gpio_irq_level { |
There was a problem hiding this comment.
I wonder if it might make sense to move some of these enums to src/common/ to prevent having to maintain separate copies in both src/rp2_common/ and src/host/? (But perhaps that'd be more effort than it's worth? 🤷 )
There was a problem hiding this comment.
I agree but I didn't want to make too many assumptions. I think there are a lot of enums and #defines that could theoretically be moved to src/common/ to aid additional implementations under src/host/ (e.g. pio_instructions.h).
Happy to make any changes though.
There was a problem hiding this comment.
I'll leave this up to the pico-sdk maintainers 🙂
|
|
||
| // debugging | ||
| #ifndef PICO_DEBUG_PIN_BASE | ||
| #define PICO_DEBUG_PIN_BASE 19u |
There was a problem hiding this comment.
For completeness, maybe you also want PICO_DEBUG_PIN_COUNT ?
There was a problem hiding this comment.
Yes, good point. I think all the 'debug pin' lines can be taken from the rp2_common header.
2 participants
As stated in #2736, the host GPIO implementation is currently a little too basic and is missing quite a few functions that have been introduced in recent SDK versions.
This PR has several commits to make the implementation a bit more usable for running tests on the host.